Skip to content

Geo location trigger added#16967

Merged
balloob merged 9 commits intohome-assistant:devfrom
exxamalte:zone-trigger-with-entity-id-pattern
Oct 22, 2018
Merged

Geo location trigger added#16967
balloob merged 9 commits intohome-assistant:devfrom
exxamalte:zone-trigger-with-entity-id-pattern

Conversation

@exxamalte
Copy link
Copy Markdown
Contributor

@exxamalte exxamalte commented Sep 29, 2018

Description:

This is a new geo_location trigger which is very similar to the zone trigger. The major difference though is that this trigger supports a source attribute defined by the platform. This allows an automation to be triggered for entities where the exact entity id is not known at the time the automation is defined.

This is in particular important for the new geo_location component where platforms automatically generate and remove entities based on external events. Each platform will define its unique source which can then be used by this trigger.
Additionally, conditions (for example based on attribute values of the entity) can be used to limit the automation being triggered.

Related issue (if applicable):

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#7041

Example entry for configuration.yaml (if applicable):

automation:
  trigger:
    platform: geo_location
    entity_id: 'geo_location.nsw_fire_service_*'
    zone: zone.bushfire_alert_zone
  action:
    - service: persistent_notification.create
      data_template:
        message: '{{ trigger.to_state.name }}'
        title: "Fire Alert"

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@exxamalte
Copy link
Copy Markdown
Contributor Author

I plan to make updates to the documentation as the PR progresses.

@exxamalte exxamalte force-pushed the zone-trigger-with-entity-id-pattern branch from e31b8fa to 42c6da7 Compare September 29, 2018 16:24
@balloob
Copy link
Copy Markdown
Member

balloob commented Oct 1, 2018

I would suggest a new PR that treats removal of entity as a leave event for a zone to be 👍

vol.Any(EVENT_ENTER, EVENT_LEAVE),
}),
# Make sure either entity id or entity id pattern are defined.
cv.has_at_least_one_key(CONF_ENTITY_ID, CONF_ENTITY_ID_PATTERN)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make this exclusive, since that's also how it's coded right now ;)

return async_track_state_change(hass, entity_id or entity_id_pattern,
zone_automation_listener,
MATCH_ALL, MATCH_ALL,
entity_id_with_pattern=bool(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a good addition to the track state change helper, it's not common enough (yet).

Instead I suggest that you just add your own event listener for STATE_CHANGED_EVENTS inside this trigger.

from homeassistant.helpers import (
condition, config_validation as cv, location)

CONF_ENTITY_ID_PATTERN = 'entity_id_pattern'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure yet if a pattern is the right solution here, it's making things complicated, especially for the UI. I wonder if a new geo_location automation trigger would be better.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, let me look into making this a separate trigger. I would still allow to define a pattern though. And I could look at solving the leave event at the same time.

@exxamalte exxamalte force-pushed the zone-trigger-with-entity-id-pattern branch from 42c6da7 to 263e4ae Compare October 4, 2018 14:37
@exxamalte exxamalte changed the title Zone trigger supports entity id pattern Geo location trigger added Oct 4, 2018
@exxamalte
Copy link
Copy Markdown
Contributor Author

Alright, instead of extending the zone trigger, I now implemented a simple new geo_location trigger that supports an entity id pattern.


TRIGGER_SCHEMA = vol.Schema({
vol.Required(CONF_PLATFORM): 'geo_location',
vol.Required(CONF_ENTITY_ID): cv.string,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I keep coming back to the pattern and not liking it. I wonder if we should drop this and instead get geo location entities to add a source/platform attribute?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because now I look at this and it's so similar to the zone one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is indeed very similar to the zone trigger.

Ultimately, what I want to achieve is to trigger an automation based on an entity in the geo_location domain where (unlike a device tracker) the specific entity id is unknown at the time the trigger is defined.
I would assume that all entities that should trigger the automation are originating from the same source. The entity_namespace combined with an entity id pattern was an obvious and simple choice for me. Alternatively, we could add a custom defined source to each platform definition and modify this trigger to act on all entities with the same source.

Example:

geo_location:
  - platform: nsw_rural_fire_service_feed
    source: 'bushfires'
    categories:
      - 'Emergency Warning'
      - 'Watch and Act'
      - 'Advice'

automation:
  trigger:
    platform: geo_location
    source: 'bushfires'
    zone: zone.bushfire_alert_zone
  action:
    - service: persistent_notification.create
      data_template:
        message: '{{ trigger.to_state.name }}'
        title: "Fire Alert"

Each platform could also define some default source (equal to the platform name). I am not in favour of just using the platform's name because that would make it impossible to override and customise this value.
In the above example a user may want to have entries matching category 'Emergency Warning' as a separate source to trigger a different automation than those entries matching 'Advice'.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we standardize how each platform for geo location exposes their info, we could have a similar structure for the trigger.

So besides source we can also define categories ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The categories are just an example. This particular feed defines 4 distinct categories that I am using in this platform for filtering. Not all feeds though define categories.
If for example the feed entries represent earthquakes, I would imagine that the magnitude is a defining attribute that can be used for filtering, and for defining different automations depending on the magnitude value.

How about I add a new method to GeoLocationEvent:

class GeoLocationEvent(Entity):
...
    @property
    def source(self) -> Optional[str]:
        """Return source value of this external event."""
        return None

And then each platform can override this value, for example in nsw_rural_fire_service_feed.py:

PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
...
    vol.Optional('source', default='nsw_rural_fire_service'): cv.string,
})

def setup_platform(hass, config, add_entities, discovery_info=None):
...
    source = config['source']
    # pass source down to NswRuralFireServiceLocationEvent...
...

class NswRuralFireServiceLocationEvent(GeoLocationEvent):

    @property
    def source(self) -> Optional[str]:
        """Return source value of this external event."""
        return self._source

And then, for this PR, instead of matching an entity id pattern, I would check if the entity that had a state change actually has a source attribute (and longitude and latitude?).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible that each source can start adding categories ? So for earthquake the categories could be the magnitude (1, 2, 3 etc)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of earthquakes I would imagine that users probably want to define a magnitude threshold, i.e. if magnitude >= 4.5 then send notification. So, in this case, it would not translate well into distinct categories.
The magnitude can certainly be an attribute of each entity, and then be used in a condition.

Similarly, in platforms that define textual categories (like 'Emergency Warning', 'Watch and Act', etc.) the category could be provided in an attribute and again used in a condition.
Or, if the external feed for example has a "last updated" date and time, this information could be used in a condition.

So, coming back to the trigger, was your thinking to allow the user to define a trigger based on the category that the platforms define?

automation:
  trigger:
    platform: geo_location
    category: 'Emergency Warning'
    zone: zone.bushfire_alert_zone
...
automation:
  trigger:
    platform: geo_location
    category: 4.5
    zone: zone.earthquake_alert_zone
...

I think the category alone would not be sufficient, and at least the source platform name (or a user-defined value) would be required additionally. For example:

automation:
  trigger:
    platform: geo_location
    source: 'nsw_rural_fire_service_feed'
    category: 'Emergency Warning'
    zone: zone.bushfire_alert_zone
...

But as explained above, you could then as well just add a condition to limit the automation to a certain category (or other defining attribute). This approach would also be easier for attributes types float, date, etc. For example:

automation:
  trigger:
    platform: geo_location
    source: 'nsw_rural_fire_service_feed'
    zone: zone.bushfire_alert_zone
  condition:
    condition: template
    value_template: "{{ trigger.to_state.attributes.category == 'Emergency Warning' }}"
...

I think there are some limitations though, combining convenience functions like state_attr or is_state_attr with the trigger variables.

Copy link
Copy Markdown
Member

@balloob balloob Oct 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I forgot about conditions.

We hardcode source to be the source of the platform. Triggers will just trigger based on the source and if the event is in a zone.

Users can then filter out specific ones using conditions: https://www.home-assistant.io/docs/scripts/conditions/#numeric-state-condition

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries.
I'll implement a simple source for the existing platforms in a separate PR, and then come back to this one and update the trigger with the new logic.
I think conditions should work very well then, especially if the platforms provide reasonably useful information in the form of attributes.

TRIGGER_SCHEMA = vol.Schema({
vol.Required(CONF_PLATFORM): 'geo_location',
vol.Required(CONF_SOURCE): cv.string,
vol.Required(CONF_ZONE): cv.entity_id,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entity_domain('zone')

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

zone_entity_id = config.get(CONF_ZONE)
trigger_event = config.get(CONF_EVENT)

def source_match(state):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this outside of async_trigger

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

'zone': zone_state,
'event': trigger_event,
},
}, context=None if not to_state else to_state.context))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

context=event.context

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

@exxamalte exxamalte force-pushed the zone-trigger-with-entity-id-pattern branch from 40e6610 to 4936060 Compare October 13, 2018 01:49
@balloob balloob merged commit 75e42ac into home-assistant:dev Oct 22, 2018
@balloob balloob added this to the 0.81 milestone Oct 22, 2018
@ghost ghost removed the in progress label Oct 22, 2018
@exxamalte
Copy link
Copy Markdown
Contributor Author

Thanks, I'll get onto the documentation asap.

balloob pushed a commit that referenced this pull request Oct 23, 2018
* zone trigger supports entity id pattern

* fixed lint error

* fixed test code

* initial version of new geo_location trigger

* revert to original

* simplified code and added tests

* refactored geo_location trigger to be based on a source defined by the entity

* amended test cases

* small refactorings
@exxamalte
Copy link
Copy Markdown
Contributor Author

Documentation PR now added home-assistant/home-assistant.io#7041

@balloob balloob mentioned this pull request Oct 26, 2018
iantrich pushed a commit to iantrich/home-assistant that referenced this pull request Oct 27, 2018
* zone trigger supports entity id pattern

* fixed lint error

* fixed test code

* initial version of new geo_location trigger

* revert to original

* simplified code and added tests

* refactored geo_location trigger to be based on a source defined by the entity

* amended test cases

* small refactorings
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
@exxamalte exxamalte deleted the zone-trigger-with-entity-id-pattern branch September 27, 2019 08:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants